Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: chectl improvements #948

Merged
merged 1 commit into from
Nov 2, 2020
Merged

fix: chectl improvements #948

merged 1 commit into from
Nov 2, 2020

Conversation

tolusha
Copy link
Collaborator

@tolusha tolusha commented Oct 22, 2020

Signed-off-by: Anatolii Bazko abazko@redhat.com

What does this PR do?

  • initialize context for all commands in the same way
  • fix 'openshift is runningcheck to runoc status` for default namespace
  • add -y flag for confirmation, others flag marked as hidden.
  • make server:update command platform independent (the corresponding flags are hidden)

PR Checklist

As the author of this Pull Request I made sure that:

Reviewers

Reviewers, please comment how you tested the PR when approving it.

src/api/kube.ts Outdated Show resolved Hide resolved
}),
'skip-kubernetes-health-check': skipKubeHealthzCheck
'skip-kubernetes-health-check': skipKubeHealthzCheck,
yes: assumeYes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assume is more general then confirmation

src/commands/server/delete.ts Outdated Show resolved Hide resolved
src/commands/server/delete.ts Outdated Show resolved Hide resolved
}),
platform: string({
char: 'p',
description: 'Type of Kubernetes platform. Valid values are \"minikube\", \"minishift\", \"k8s (for kubernetes)\", \"openshift\", \"crc (for CodeReady Containers)\", \"microk8s\".',
options: ['minikube', 'minishift', 'k8s', 'openshift', 'microk8s', 'docker-desktop', 'crc'],
hidden: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it a required parameter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is not required.
We hide it since it isn't used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe, then, we have to delete it at all?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep it for backward compatibility

src/commands/server/update.ts Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: AndrienkoAleksandr, mmorhun, tolusha
To complete the pull request process, please assign after the PR has been reviewed.
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@AndrienkoAleksandr
Copy link
Contributor

/retest

@AndrienkoAleksandr AndrienkoAleksandr changed the title chectl improvements fix: chectl improvements Oct 23, 2020
@openshift-ci-robot
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@tolusha tolusha force-pushed the someimprovements branch 2 times, most recently from 463e025 to 3b82471 Compare October 30, 2020 08:32
@tolusha
Copy link
Collaborator Author

tolusha commented Oct 30, 2020

/test v4-chectl-e2e

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@tolusha tolusha merged commit 9304a68 into master Nov 2, 2020
@tolusha tolusha deleted the someimprovements branch November 2, 2020 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants